Parameterize N1QL queries with positional parameters#34
Conversation
Add serialize_for_binding(value) to convert Ruby types for N1QL binding (Time/DateTime → ISO8601, Date → string, others pass through). Add bind(value, params) to push values into a positional parameter array and return the $N placeholder. Add params: keyword argument to build_match, build_match_hash, build_match_range, and build_not_match. When params: is provided, values are bound via bind(); otherwise falls back to quote() for backward compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
build_where now accepts a params: keyword and binds the type filter and emit_key values as positional parameters ($1, $2, ...) instead of interpolating them into the query string. run_query creates a params array, passes it through build_where, and forwards positional_parameters to Couchbase::Options::Query. The debug log switches to a block form and includes the params array. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace string-interpolated values with positional parameters in all Relation query paths: - Add to_n1ql_with_params returning [query, params] - Add build_where_with_params, build_conds_with_params using bind() - Add build_update_with_params using bind() for both scalar and Hash values (fixes the injection vulnerability from build_update) - Add build_query_options helper for Couchbase::Options::Query - Update execute, query, first, last, update_all to use params Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace string interpolation in build_index_n1ql with positional parameters ($1 for type, $2 for foreign key value), enabling prepared-statement caching for has_many :through queries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hing Instead of expanding arrays into individual parameters (e.g. IN [$1, $2]), pass the whole array as one parameter (e.g. IN $1). This keeps the query string stable regardless of array length, enabling Couchbase to cache the prepared statement for IN queries. serialize_for_binding now handles arrays recursively. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TimeWithZone does not inherit from Time or DateTime, so it was not being serialized to ISO8601. Use acts_like?(:time) duck-typing to handle all time-like objects from Rails. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces parameterized queries to the Couchbase ORM to replace raw string interpolation, enhancing security against N1QL injection and improving query performance. It updates query-building methods across relations, N1QL execution, and helpers to bind values into positional parameters. The review feedback highlights two important issues: first, a mismatch in spec/relation_spec.rb where the test assertions for array parameterization do not align with the actual implementation (which binds the entire array as a single parameter); second, a bug in QueryHelper#bind where returning nil for nil values leads to invalid N1QL syntax during update_all operations instead of binding nil as a positional parameter.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| expect(n1ql).to include("name IN [$2, $3]") | ||
| expect(params).to eq(["relation_model", "Alice", "Bob"]) |
There was a problem hiding this comment.
The test asserts that array IN conditions are parameterized as name IN [$2, $3] with individual elements in params. However, the implementation in QueryHelper#build_match binds the entire array as a single positional parameter (name IN $2 with params containing ["relation_model", ["Alice", "Bob"]]), which is also the intended behavior described in the PR description ('Pass arrays as single positional parameter'). This mismatch will cause the test to fail. Update the assertions to match the actual implementation.
expect(n1ql).to include("name IN $2")
expect(params).to eq(["relation_model", ["Alice", "Bob"]])| def bind(value, params) | ||
| if value.nil? | ||
| nil | ||
| else | ||
| params << serialize_for_binding(value) | ||
| "$#{params.length}" | ||
| end | ||
| end |
There was a problem hiding this comment.
Currently, bind returns nil when value is nil. In build_update_with_params, this causes update_all(field: nil) to generate invalid N1QL syntax like field = (with an empty value after the equals sign). Since build_match and build_not_match already handle nil values explicitly before calling bind, we can safely remove the nil check in bind so that nil is correctly bound as a positional parameter (e.g., field = $N with nil in params).
def bind(value, params)
params << serialize_for_binding(value)
"$#{params.length}"
end
1 participant
Summary
Replace string-interpolated values with positional parameter placeholders ($1, $2, ...) across all N1QL query paths. This makes query strings stable regardless of parameter values, enabling Couchbase prepared-statement caching.
Depends on: #33 (Fix N1QL injection in nested Hash)
Changes (6 commits)
serialize_for_binding/bindhelpers and parameterize query_helper — Building blocks: type conversion for binding and$Nplaceholder generation. Allbuild_match*methods accept optionalparams:keyword.build_whereandrun_queryuse positional parameters; debug log switches to block form with params.to_n1ql_with_params,build_where_with_params,build_update_with_params,build_query_options. All query paths (execute,query,first,last,update_all) use params.build_index_n1qluses$1/$2for type and foreign key.IN $1instead ofIN [$1, $2, ...]for stable query strings regardless of array length.ActiveSupport::TimeWithZone—acts_like?(:time)duck-typing inserialize_for_binding.Releasability
Queries are parameterized but
adhocremains at SDK default (true= no prepared-statement cache), so there is no behavioral change. The adhoc option will be added in a follow-up PR.Test plan
/test --fail-fast=false🤖 Generated with Claude Code